👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

• Add ready label to the PR
• Enable auto-merge.

🚀

I don't think you need to create another platform class.

if Jetson does not support nvlink, you can just detect Jetson and return false for the nvlink detection.

creating a platform just for Jetson is too overkill.

It's not just NVLink; currently that's only used for custom allreduce as far as I can tell.

The CUDA platform class assumes NVML is available throughout to avoid initialising a CUDA context, but Jetson doesn't have NVML, so a different approach needs to be taken.

I didn't want to pollute the CUDA platform by initializing a CUDA context, so created a different one.

Happy to rework if you prefer a different approach?

Alternatively, could try to abstract away the NVML calls to make calls to an alternative backend on Jetson - jetson_stats might be usable, but not sure if all features will be implemented (especially if we want to use other NVML features in future).

I believe Jetson is also currently the only CUDA-supporting device to use a unified memory configuration, so some optimisations may be possible there (e.g. handover of weights between CPU and GPU without copy).

However it could be argued to save the separate backend for when such optimisations are actually implemented.

Looking at the logs, adding 8.7 to the ARCHS is not affecting the binary size at all:

This PR:

[2024-10-27T13:36:44Z] #28 0.789 Wheel dist/vllm-0.6.3.post2.dev123+gda79e3e5.cu124-cp38-abi3-linux_x86_64.whl is within the allowed size (184.58 MB).

Latest main:

[2024-10-28T05:09:01Z] #28 0.878 Wheel dist/vllm-0.6.3.post2.dev125+g32176fee.cu124-cp38-abi3-linux_x86_64.whl is within the allowed size (184.58 MB).

Does this make sense?

Possibly - as far as I know 8.6 and 8.7 are essentially compatible, so it's possible it's just the same kernels being linked twice?

I did have to add the 8.7 capability to get vLLM running on Orin though; without, it'd complain that there was no compatible kernel available.

Possibly - as far as I know 8.6 and 8.7 are essentially compatible, so it's possible it's just the same kernels being linked twice?

I did have to add the 8.7 capability to get vLLM running on Orin though; without, it'd complain that there was no compatible kernel available.

Ok, I didn't catch this before, but this PR isn't adding the sm87 kernels to the wheel file (just adds support for compiling them). IIUC, Jetson is ARM-only and the vLLM wheels are only built for x86, so all that looks good to me for this PR

I can find https://github.com/dusty-nv/jetson-containers/tree/master/packages/llm/vllm who builds vllm on jetson. I don't see any manipulation like this.

I can find https://github.com/dusty-nv/jetson-containers/tree/master/packages/llm/vllm who builds vllm on jetson. I don't see any manipulation like this.

Just investigated this. NVIDIA JetPack 6 includes NVML support, so the alternate platform will not be needed on JetPack 6.

However, JetPack 5 and older still do not support NVML, and JetPack 6 is only releasing for the Orin series, not any older Jetson devices. So if we want to support JetPack 5 devices we will need to use the alternate non-NVML approach.

@youkaichao 抱歉，您能重新审核一下吗？如果能尽快合并就太好了。 谢谢

Can this branch build VLLM directly without relying on Jetson containers?

Yes, I am currently building this directly on Jetson outside of containers. Build process is fully specified by my nixpkgs branch; for Jetson support make sure to override the source hash to use this branch.

You are welcome to build it directly using Nix (I am running jetpack-nixos) or adapt to whatever build scripts you are using.

Bump, still pending review

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @conroy-cheers.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@conroy-cheers can you please merge main to solve the conflict?

Merged main and retested on Jetson. Ready to go, assuming CI passes 👍

CI seems to be failing, not sure if it's related to the changes here?

@conroy-cheers failing test is unrelated I'll ask for a force merge, thanks for your work!